chore: harmonize CI workflow, templates, and test structure - #21
Conversation
Part of the fleet-wide plugin_* harmonization effort. - Pin actions/checkout, shivammathur/setup-php, actions/upload-artifact to current release commit SHAs (was floating @v4/@v2). - Stop logging the MySQL root password in CI output (cat ~/.my.cnf -> chmod 600). - Harden MySQL bootstrap: quoted --defaults-file, grants added for both 'cactiuser'@'localhost' and 'cactiuser'@'127.0.0.1'. - Add "Restore vendor ownership for Pest" step after Composer install. - Switch the PHP syntax-check step to the vendor-excluding, null-delimited find/xargs pattern used in plugin_audit. - Remove the unused "Configure Apache" step (apache2/libapache2-mod-php are not installed and not needed). - Simplify the Pest invocation to rely solely on phpunit.xml's <testsuites>. - No CodeQL workflow added: this repo has no JavaScript/Python/Ruby content (PHP only), so there is nothing for CodeQL to scan. - Consolidate tests/Security/Php74CompatibilityTest.php into tests/Security/PhpCompatibilityTest.php, now checking for PHP 8.3/8.4-only syntax (this plugin's floor is PHP 8.2 per the CI matrix). - Remove tests/TestCase.php and its require in tests/bootstrap-unit.php: not referenced by any uses(TestCase::class) call in this plugin's tests. - Add .github/ISSUE_TEMPLATE/{bug_report,feature_request}.md and .github/PULL_REQUEST_TEMPLATE.md, styled after Cacti/cacti's own templates. - Document CI/dependency baselines in copilot-instructions.md.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new PhpCompatibilityTest.php needs the standard GPL header/clearer failure context, and the bug report issue template’s labels front matter is currently structured to apply a single combined label instead of multiple labels.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
This PR continues the fleet-wide harmonization effort for plugin_* repositories by aligning this plugin’s CI workflows, templates, and test structure with the emerging shared conventions.
Changes:
- Harmonizes CI workflows: pins Actions to commit SHAs, hardens MySQL bootstrap, simplifies Pest invocation, and removes dead workflow steps.
- Updates test structure: removes unused
tests/TestCase.phpand replaces the legacy PHP-7.4 compatibility checks with a PHP 8.2-floor compatibility test. - Adds repository templates/documentation: introduces issue/PR templates and extends
.github/copilot-instructions.mdwith CI/dependency baselines.
| File | Description |
|---|---|
tests/TestCase.php |
Removes an unused PHPUnit base class. |
tests/Security/PhpCompatibilityTest.php |
Adds a PHP-floor compatibility test scanning plugin PHP sources for PHP 8.3/8.4-only constructs. |
tests/Security/Php74CompatibilityTest.php |
Removes the old PHP 7.4-era compatibility checks. |
tests/bootstrap-unit.php |
Stops requiring the removed TestCase.php. |
.github/workflows/plugin-ci-workflow.yml |
Aligns CI steps with pinned actions, safer DB bootstrap, syntax-check pattern, and simplified Pest invocation. |
.github/workflows/codeql.yml |
Adds CodeQL workflow (JS/TS analysis only). |
.github/PULL_REQUEST_TEMPLATE.md |
Adds a PR template aligned with the fleet pattern. |
.github/ISSUE_TEMPLATE/feature_request.md |
Adds a feature request template. |
.github/ISSUE_TEMPLATE/bug_report.md |
Adds a bug report template. |
.github/copilot-instructions.md |
Documents CI/dependency baselines for Copilot guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Document the locales/build_gettext.sh + cacti.pot workflow in copilot-instructions.md (Weblate owns per-language .po/.mo sync). - Add a CI step that regenerates cacti.pot and fails if it is stale, ignoring the POT-Creation-Date timestamp. - Mark locales/build_gettext.sh executable. - Bump the CI MariaDB service image floor to 11.8 (from mariadb:10.6 / mysql:8.0).
- mariadb:11.8 no longer ships mysqladmin; use mariadb-admin for the service container healthcheck. - Run chmod/build_gettext.sh with sudo in the i18n verification step, since locales/ is owned by www-data by the time it runs.
Ran the real locales/build_gettext.sh (xgettext/msgmerge/msgfmt) to refresh the translation template against current source. Per the i18n workflow documented in copilot-instructions.md, only the regenerated cacti.pot is committed here; Weblate owns syncing the per-language .po/.mo files from it.
…ibilityTest Matches the project's PSR coding standard (short array syntax, single quotes for non-interpolated strings) that php-cs-fixer enforces in CI.
`find` without `sort` returns filesystem/readdir-order results, which can differ between machines (e.g. a local regen vs. a GitHub Actions runner), producing a spurious reordering diff in cacti.pot even when no strings actually changed. Pipe through `sort` and regenerate cacti.pot with the now-deterministic order.
- tests/Security/PhpCompatibilityTest.php: fixed the #[Override]/#[Deprecated] attribute regexes to also match the fully qualified (`#[\Override]`, `#[\Deprecated]`) form, added a realpath() false-guard for the plugin root, included the relative file path in both RuntimeException messages, added typed-class-constant and dynamic-class-constant-fetch checks (PHP 8.3), and restored each()/create_function() removed-in-PHP-8.0 guards that the consolidation had dropped. Also excludes include/vendor/ (not just vendor/) from the recursive source scan. - .github/workflows/plugin-ci-workflow.yml: the "Create MySQL Config" step's root password is now masked via `::add-mask::` before it's echoed into ~/.my.cnf, so it no longer appears in plaintext in the Actions log (chmod 600 alone only protected the file after creation, not the command's own echoed source in the log).
|
This PR's |
|
Acknowledged, left as-is: verified against Cacti/cacti's own actual |
The each()-removed-in-PHP-8.0 check matched jQuery's $.each(/.each( calls too, which have nothing to do with the removed PHP global function. Exclude any each( preceded by . or > via a negative lookbehind.


Description
Part of the fleet-wide
plugin_*CI/template/test-structure harmonizationeffort (see plugin_analytics#8, plugin_apcupsd#29).
Changes
actions/checkout,shivammathur/setup-php,actions/upload-artifactnow pinned to current release commit SHAs.cat ~/.my.cnfprinted the root passwordinto the Actions log; replaced with
chmod 600.--defaults-file, grants added forboth
'cactiuser'@'localhost'and'cactiuser'@'127.0.0.1'.step after Composer install, matching the pattern in
plugin_apcupsd.find/xargspattern fromplugin_audit.never installed in this workflow, so the step was dead weight.
phpunit.xml's own<testsuites>block instead of duplicating the test directory list.test replaced with
tests/Security/PhpCompatibilityTest.php, checking forPHP 8.3/8.4-only syntax (this plugin's floor is PHP 8.2 per the CI matrix).
tests/TestCase.php: not referenced by anyuses(TestCase::class)call in this plugin's actual tests.Related Issue
N/A — internal fleet harmonization, not tracked against a specific issue.
How Has This Been Tested?
Types of changes
Checklist
out across
plugin_*repos.